Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sapphire - Angie Contreras, Lyuda Kim #10

Open
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

lyudarkim
Copy link

No description provided.

celina-barron added a commit to celina-barron/viewing-party that referenced this pull request Mar 30, 2023
…passing all but Ada-C19#9, Celina's implementation (commented out) passing all but Ada-C19#9 and Ada-C19#10
Copy link

@audreyandoy audreyandoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work Angie and Lyuda! You hit all the learning goals for this project and all tests are passing. You have earned a well-deserved 🟢 grade on this project ✨

I added comments, compliments, and hints on ways to refactor your code.

Keep up the great work! ✨

Comment on lines 5 to +16
def create_movie(title, genre, rating):
pass
# Input validation
if not title or not genre or not rating:
return None

movie = {
"title": title,
"genre": genre,
"rating": rating
}
# Return the dictionary
return movie

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Great use of a guard clause to exit the function early. Optionally, we could return the dictionary literal itself to save 1 line (a really minor nitpick).

def create_movie(title, genre, rating):
    # Input validation
    if not title or not genre or not rating:
        return None

    return {
        "title": title,
        "genre": genre,
        "rating": rating
    }

Comment on lines +18 to +32
def add_to_watched(user_data, movie):
if not movie:
return user_data

user_data["watched"].append(movie)

return user_data

def add_to_watchlist(user_data, movie):
if not movie:
return user_data

user_data["watchlist"].append(movie)

return user_data

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Great work checking if the movie was valid first before adding to the watched/watchlist lists!

Comment on lines +34 to +46
def watch_movie(user_data, title):

for i in range(len(user_data['watchlist'])):
title_from_dict = user_data['watchlist'][i]['title']

# If title is in watchlist, remove that movie from watchlist
if title_from_dict == title:
dict_from_list = user_data['watchlist'][i]
user_data["watchlist"].remove(dict_from_list)
# Add that movie to "watched"
user_data["watched"].append(dict_from_list)

return user_data

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice work!

One thing to consider is that a ranged loop is really useful when we need to swap items in a list and/or utilize the indices in a list. Because this feature primarily focuses on the values of the list ( the movie dictionaries), we can iterate through the list directly instead of indices (typically what a ranged loop like this is used for). Iterating over the items directly omits an index variable(which we aren't really utilizing anywhere within this function, we're only checking the value of the movie's title compared to the title that was passed to the function).

This suggestion would look like:

def watch_movie(user_data, title):

    for movie in user_data['watchlist']:
    # If title is in watchlist, remove that movie from watchlist
        if movie["title"] == title:
            user_data["watchlist"].remove(movie)
            # Add that movie to "watched"
            user_data["watched"].append(movie)
            break
    return user_data

I also have added a break to prevent unnecessary iterations (For example, if the matching movie is found in the first iteration, we can break out of the loop early instead of having to iterate through the rest of the list.)

Comment on lines +52 to +63
def get_watched_avg_rating(user_data):

if user_data["watched"] == []:
return 0.0

total_rating = 0

for i in range(len(user_data["watched"])):
rating_from_dict = user_data['watched'][i]['rating']
total_rating += rating_from_dict
num_of_movies = len(user_data["watched"])
return total_rating / num_of_movies

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Comment on lines +66 to +86
def get_most_watched_genre(user_data):
genres_most_watched = {}

# Iterate through "watched" list
for i in range(len(user_data['watched'])):
genre = user_data['watched'][i]['genre']
if genre in genres_most_watched:
genres_most_watched[genre] += 1
else:
genres_most_watched[genre] = 1

highest_occurence = 0
most_watched = None

# Iterate through out genres_most_watched dict
for genre, num in genres_most_watched.items():
if num > highest_occurence:
highest_occurence = num
most_watched = genre

return most_watched

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice use of a frequency map!

Comment on lines +169 to +170
for i in range(len(user_watched)):
user_watched_titles.append(user_watched[i]["title"])

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 This would be great to add to a helper function!

Comment on lines +173 to +182
for dict in friends_list:
for value in dict.values():
for movie in value:
friends_title = movie["title"]

for user_dict in user_watched:
if friends_title not in user_watched_titles and movie["host"] in user_data["subscriptions"] and movie not in movie_recs:
movie_recs.append(movie)

return movie_recs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A quadruple loop! I highly recommend trying to see if we can reduce the number of nested loops. Tools like VS Code Debugger can be really helpful in visualizing the shape of the data we're working with.

Comment on lines +188 to +227
def get_new_rec_by_genre(user_data):
# New dict will hold the most watched genre
user_watched_list = user_data["watched"]
most_watched_genre = {}
for movie in user_watched_list:
value_genre = movie["genre"]
if value_genre in most_watched_genre:
most_watched_genre[value_genre] += 1
else:
most_watched_genre[value_genre] = 1

# Max_watched_genre hold the value of the most watched genre by user
max_watched_genre = ""
max_watched = 0
for genre, num in most_watched_genre.items():
if num > max_watched:
max_watched = num
max_watched_genre = genre

# Create a dict that'll hold the titles of user's watched movies
user_watched_movies = {}
for movie in user_watched_list:
key = movie["title"]
user_watched_movies[key] = 1

# List of recommended movies by most watched genre and user hasn't watched it.
rec_movies_by_gender = {}
friends_list = user_data["friends"]
for dict in friends_list:
for value in dict.values():
for movie in value:
watched_by_friend = movie["title"]
genre_movie_friend = movie["genre"]
# La agrego si yo no la he visto y si es del genero que mas veo
if watched_by_friend in rec_movies_by_gender:
continue
if watched_by_friend not in user_watched_movies and genre_movie_friend == max_watched_genre:
rec_movies_by_gender[watched_by_friend] = movie

return list(rec_movies_by_gender.values())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How would this function change if we used get_most_watched_genre as a helper function?

Comment on lines +230 to +257
def get_rec_from_favorites(user_data):

# List that holds the titles of the favorite movies.
favorites_list = user_data["favorites"]
friends_titles = []

# Access the list of movies that friends' have watched
friends_watched_list = user_data["friends"]

# Compare every item in the favorites list against the movies in friends_watched_list
# Nested loops to access every title in friends_watched_list
result_favs = []
for dict in friends_watched_list:
for value in dict.values():
for movie_watched in value:
title = movie_watched["title"]

if title not in friends_titles:
friends_titles.append(title)

# Loop through Favorites list to compare titles
for movie in favorites_list:
favorite_title = movie["title"]
# Check if the movie is in the user's favorites and None of its friends have watched
if favorite_title not in friends_titles and movie not in result_favs:
result_favs.append(movie)

return result_favs

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! How might this function change if we used get_friends_unique_watched and get_most_watched_genre as helper functions?

Comment on lines +187 to +188
assert updated_data == {'watchlist': [{'title': 'The Lord of the Functions: The Fellowship of the Function', 'genre': 'Fantasy', 'rating': 4.8}], 'watched': [{'title': 'The Lord of the Functions: The Two Parameters', 'genre': 'Fantasy', 'rating': 4.0}, {'title': 'It Came from the Stack Trace', 'genre': 'Horror', 'rating': 3.5}]}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants